-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Make Definitions survive recompilation of core definitions. #928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
c0b6351 to
a384d83
Compare
|
/rebuild |
1 similar comment
|
/rebuild |
3407494 to
73ff369
Compare
|
Rebased to master. |
|
/rebuild |
|
Current status: The stdlib slice in scala-collections-whitelist now compiles, except for #912 and #916. |
|
@odersky is this PR ready for review? |
/rebuild |
|
/rebuild |
1 similar comment
|
/rebuild |
|
@DarkDimius Ready for review now. |
Symbols are not stable between runs, so if some symbol referred to from Definitions gets recompiled, there are then two Symbols that are both visible, one referenced from Definitions, the other the one that got compiled. Thos led to a crash when e.g. compiling scala.Short, because the newly compiled symbol was not recognized as a primitive value class. The present commit tries to make systematic changes without regard to simplicity or aesthetics. This will be polished in future commits. // ### comments signal areas that need further attention.
TypeRefs can have several representations for logically the same type, so they don't make good keys.
Also, delete unused `uncheckedStableClassRef` entry.
Can't be a lazy val, because one of the symbols it tests (`newRefArray`) can be recomputed.
Remve versions in Symbols, always go through version in Denotations. Avoids having two equivalent ways to do the same thing.
Was a lazy val, but this is not stable under recompilation.
Maps should not have TypeRefs as keys, yet symbols are not stable for recompilation. Solution: Construct these maps over symbols but move them to CapturedVars#Transform where they will be re-built on each run.
Remove unneeded maps and make the ones that are needed private. Reason: Maps indexed with TypeNames are prone to misuse; should be accessed only with names of known primitive classes.
Contains tests on thsoe sets would be flakey anyway.
1) Have symbol sets cached per run 2) Use methods Denotation#isPrimitiveValueClass, Denotation#isNumericValueClass instead of calling contains directly on symbol sets.
TypeRef becomes Type, thus removing duplicates. Where ...Type was used in an extraction (e.g. ArrayType(...), FunctionType(...)), we now use ...Of.
Since we now have two forms of (almost) everything in Definitions, might as well profit from it.
Again it dies without an exception trace. Is partest eating error output?
Now also provides compilation unit.
Track starts and ends of completions using indentation.
Forces a bit less, and could be more efficient. Did not seem to make a difference with current CyclicReferences though, except that cyclic error happened a bit later in the sequence.
Set info early in order to avoid cyclic reference errors.
Errors were observed when compiling
scala/Predef.scala scala/package.scala scala/collection/GenSeqLike.scala
Was stdout, but this gets mixed up with the exception printing on stderr.
523f4d4 to
b112b2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could defn.ProductNType(n) be used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because it would load the class. That's what caused the datarace after all.
|
/rebuild |
1) Check that searched scope is consistent 2) Do a linear search for symbol with name, and report if something was found that way.
|
/rebuild |
2 similar comments
|
/rebuild |
|
/rebuild |
Make Definitions survive recompilation of core definitions.
No description provided.